Move examples/models/... out of the `torch` namespace #5318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Closed

dbort wants to merge 1 commit into pytorch:main from dbort:export-D62591348

Contributor

dbort commented Sep 12, 2024

Summary:
The code under examples/... is a proxy for user code, and users should never declare code under the torch:: or executorch:: namespaces.

Move this code under the example:: namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use enum class to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Differential Revision: D62591348

pytorch-bot bot commented Sep 12, 2024 •

edited

Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/5318

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 5 New Failures, 1 Cancelled Job, 1 Unrelated Failure

As of commit 39b248c with merge base 67be84b ():

NEW FAILURES - The following jobs have failed:

Apple / build-demo-ios / macos-job (gh)
RuntimeError: Command bash /Users/runner/work/_temp/exec_script failed with exit code 1
trunk / test-models-macos (cmake, add, xnnpack-quantization-delegation, macos-m1-stable, 90) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 1
trunk / test-models-macos (cmake, ic3, xnnpack-quantization-delegation, macos-m1-stable, 90) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 1
trunk / test-models-macos (cmake, mv3, xnnpack-quantization-delegation, macos-m1-stable, 90) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 1
trunk / test-models-macos (cmake, w2l, xnnpack-quantization-delegation, macos-m1-stable, 90) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

trunk / test-models-macos (cmake, softmax, portable, macos-m1-stable, 90) / macos-job (gh)
##[error]The operation was canceled.

FLAKY - The following job failed but was likely due to flakiness present on trunk:

Apple / upload-frameworks-ios (gh) (detected as infra flaky with no log or failing log classifier)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot added the CLA Signed label

Contributor

facebook-github-bot commented Sep 12, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

facebook-github-bot added the fb-exported label

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from 0e125dc to ada5783 Compare

September 13, 2024 02:17

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

ada5783

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Differential Revision: D62591348

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from ada5783 to fc13f26 Compare

September 13, 2024 16:14

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

fc13f26

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Differential Revision: D62591348

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

fcf9102

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from fc13f26 to fcf9102 Compare

September 13, 2024 16:55

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

2b56aa3

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from fcf9102 to 2b56aa3 Compare

September 13, 2024 17:13

larryliu0820 approved these changes

View reviewed changes

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

a899417

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from 2b56aa3 to a899417 Compare

September 13, 2024 17:34

dbort added the ciflow/trunk label

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from a899417 to 304b4ba Compare

September 13, 2024 18:16

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

304b4ba

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

Differential Revision: D62591348

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from 304b4ba to 16def7e Compare

September 13, 2024 22:49

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

16def7e

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

Differential Revision: D62591348

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

8558bb9

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from 16def7e to 8558bb9 Compare

September 13, 2024 22:54

Contributor

facebook-github-bot commented Sep 13, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort added a commit to dbort/executorch that referenced this pull request


          Move examples/models/... out of the torch namespace (pytorch#5318)

d56e800

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Reviewed By: larryliu0820

Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from 8558bb9 to d56e800 Compare

September 13, 2024 23:27


          Move examples/models/... out of the torch namespace (pytorch#5318)

39b248c

Summary:
Pull Request resolved: pytorch#5318

The code under examples/... is a proxy for user code, and users should never declare code under the `torch::` or `executorch::` namespaces.

Move this code under the `example::` namespace to make it more clear that users should use their own namespaces when writing code like this.

I made one non-mechanical change in llama_tiktoken.h: we should always use `enum class` to avoid polluting the parent namespace, and enum values should follow UpperSnakeCase.

Tests:
Should be a no-op; CI passes.

Prints no output:
```
find examples/models -type f | xargs grep "namespace torch"
find examples/models -type f | xargs grep "namespace executorch"
```

Build llava_main:
```
bash .ci/scripts/test_llava.sh
```

Reviewed By: larryliu0820

Differential Revision: D62591348

Contributor

facebook-github-bot commented Sep 14, 2024

This pull request was exported from Phabricator. Differential Revision: D62591348

dbort force-pushed the export-D62591348 branch from d56e800 to 39b248c Compare

September 14, 2024 00:29

facebook-github-bot closed this in

262dfc0

facebook-github-bot added the Merged label

Contributor

facebook-github-bot commented Sep 14, 2024

This pull request has been merged in 262dfc0.

dbort added a commit to dbort/executorch that referenced this pull request


          Fix phi-3-mini build

3230f34

Fix the build errors I introduced in pytorch#5318

Test Plan:
Build the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

dbort mentioned this pull request

Fix phi-3-mini build #5513

Closed

dbort added a commit to dbort/executorch that referenced this pull request


          Fix phi-3-mini build

896d4dc

Fix the build errors I introduced in pytorch#5318

Test Plan:
Build the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

facebook-github-bot pushed a commit that referenced this pull request


          Fix phi-3-mini build (#5513)

Summary:
Fix the build errors I introduced in #5318

Pull Request resolved: #5513

Test Plan: Built the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

Reviewed By: helunwencser

Differential Revision: D63133231

Pulled By: dbort

fbshipit-source-id: 81d5402e76fd86c919a13a1b1b83687feca72ab7

pytorchbot pushed a commit that referenced this pull request


          Fix phi-3-mini build (#5513)

a19a059

Summary:
Fix the build errors I introduced in #5318

Pull Request resolved: #5513

Test Plan: Built the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

Reviewed By: helunwencser

Differential Revision: D63133231

Pulled By: dbort

fbshipit-source-id: 81d5402e76fd86c919a13a1b1b83687feca72ab7
(cherry picked from commit 8618607)

pytorchbot mentioned this pull request

Fix phi-3-mini build #5522

Merged

jackzhxng pushed a commit that referenced this pull request


          Fix phi-3-mini build (#5522)

d770fe8

Fix phi-3-mini build (#5513)

Summary:
Fix the build errors I introduced in #5318

Pull Request resolved: #5513

Test Plan: Built the model using the instructions at https://github.com/pytorch/executorch/blob/main/examples/models/phi-3-mini/README.md

Reviewed By: helunwencser

Differential Revision: D63133231

Pulled By: dbort

fbshipit-source-id: 81d5402e76fd86c919a13a1b1b83687feca72ab7
(cherry picked from commit 8618607)

Co-authored-by: Dave Bort <[email protected]>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed fb-exported Merged